Repository navigation
Conversation
…oncierge, disconnect flash, and Back after refresh on settings pages Signed-off-by: krishna2323 <[email protected]>
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
|
@JmillsExpensify @trjExpensify @lakchote Two questions before I open it for review:
Which do you prefer?
|
…an integration turns Connections on Signed-off-by: krishna2323 <[email protected]>
I'm fine with this, so long as we go through staging regression testing first. What we want to avoid is deploying it so customers see the consolidated page and then reverting it.
Yeah, let's do this. We could have the link open on the |
Signed-off-by: krishna2323 <[email protected]>
|
Hey, I noticed you changed If you want to automatically generate translations for other locales, an Expensify employee will have to:
Alternatively, if you are an external contributor, you can run the translation script locally with your own OpenAI API key. To learn more, try running: npx bun ./scripts/generateTranslations.ts --helpTypically, you'd want to translate only what you changed by running |
|
@MelvinBot review |
ReviewThe PR looks solid overall. I found one real regression to fix before merge, and a few small issues. The four fixes in the description all check out in the code. Should fix
Minor
What I checked
|
…liation Back on the stack, and only skip the connections fetch for a workspace still being created Signed-off-by: krishna2323 <[email protected]>
…anel where an integration is connected, the Accounting tab otherwise Signed-off-by: krishna2323 <[email protected]>
|
@MelvinBot review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Follow-up reviewI reviewed the two new commits. They fix four of my earlier points and introduce no new bugs. Two points are still open. Fixed
Still open
Optional: four callers now pass four I didn't run typecheck or test the app. |
|
My quick comments:
|
|
@situchan Updated the description and Fixed Issues. |
|
Please merge main |
|
#104320 is still reproducible. after clicking on accounting, app still opens More features Screen.Recording.2026-10-22.at.9.57.38.PM.mov |
|
Looks good except #104402 (comment). |
…s page doesn't send the user to More features before the change lands Signed-off-by: krishna2323 <[email protected]>
…nections-regressions # Conflicts: # src/pages/workspace/accounting/businessCentral/export/BusinessCentralExportPage.tsx
|
Failing unit tests isn't related to this PR. |
|
#104320 pass ✅ 104320.mov |
lakchote
left a comment
There was a problem hiding this comment.
Thanks @Krishna2323! Waiting a frame before opening the setup flow fixes the More features redirect, and @situchan's retest passed. LGTM.
|
🚧 lakchote has triggered a test Expensify/App build. You can view the workflow run here. |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
🚀 Deployed to staging by https://github.com/lakchote in version: 9.5.21-0 🚀
|
|
No help site update is needed: every change in this PR fixes regressions on the Connections page, and that page is only visible behind the |
|
🚀 Deployed to staging by https://github.com/lakchote in version: 9.5.23-0 🚀
|
|
No help site update is needed: every change in this PR fixes regressions on the Connections page, which is still only visible behind the |
|
🚀 Deployed to production by https://github.com/jasperhuangg in version: 9.5.23-3 🚀
Bundle Size Analysis (Sentry): |
Explanation of Change
Fixes the regressions reported after #102265. The launch (beta removal, page deletions, link updates and the onboarding text) is in #, on top of this one, so it can be reverted without these fixes.
/connectionsinstead of/accounting.Fixed Issues
$ #104013
$ #104032
$ #104319
$ #104320
$ #104029
$ #104321
PROPOSAL: N/A, follow-up to #102265
Tests
Precondition: enable the
unifiedConnectionsbeta./connections.Offline tests
Precondition: enable the
unifiedConnectionsbeta.QA Steps
Same as tests, with the
unifiedConnectionsbeta enabled.PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectioncanBeMissingparam foruseOnyxtoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.ScrollViewcomponent to make it scrollable when more elements are added to the page.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari
test_1.mp4